Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b patrickohh-add-project-value-card-3547 gh-pages
git pull https://github.com/patrickohh/website.git add-project-value-card-3547

Review availability: Weds 12/7 3-5 pm
ETA: Weds 12/7 5pm

Hi @MattPereira , I will look into making the necessary edits as soon as possible!

1. Progress: Haven't gotten a chance to look at the changes needed for this issue due to a busy week last week.
2. Blockers: Last week was just a filled schedule for me, but will take time this week to fix the needed issues.
3. Availability: 12/21 - 12/23
4. ETA: 12/23 Evening

Hey @patrickohh solid work this looks really good! 👍

Just a couple of edits if possible:

• For small to medium screen sizes, I noticed the sustainable development goal card bumps up against the edge of its container card. Perhaps the CSS could be better adjusted for this screen size range ?See Screenshot

• For mobile screen sizes, the figma from the issue shows a modified design for the sustainable development goal card See Screenshot Comparison### Figma Design For Mobile### Current Pull Request on Mobile

Made the changes for the description card to avoid touching the outer div with small to medium screen sizes, however still finding trouble in implementing the mobile design for the description card. None of my media queries seem to work in the redesign. Is there a way to replace one div with another without having the change the whole structure of the html code?

This is awesome work, @patrickohh! You developed a section that very closely matches the prototype and with logical class names and a good use of Liquid. I totally understand why you created a new div specifically for mobile as the shift in the elements' placement is tricky to implement here! That said, I think it is possible to work off just the one div you created. I played around with some of the code and got something working using this HTML structure:

<div class='sdg-description-card'>
     <h2 class='sdg-card-title title7'>Sustainable Development Goal</h2>
     <div class="sdg-description-wrapper">
          <img class='sdg-img' src='{{ page.sdg-image-src }}'/>
          <div class='sdg-description-grid-item'>{{ page.sdg }}</div>
     </div>
</div>

The main change is the wrapper around the SDG image and text. (If you have a better class name, by all means!) The SDG title, now on its own, should be more easier to manage when working on responsive design.

I'm gonna approve this PR because you did come up with a 100% valid solution, but I also know it's good practice to avoid repeated code. If you do attempt to refactor your code and get stuck, feel free to reach out on Slack!

Made some changes to use only one div, but running into an issue where the description section of the card wraps down too much from the screen size shrinking.

I concur with @jyaymie. The responsiveness from desktop down through mobile view is excellent. This is great work on a rather difficult Figma implementation

Seems like all that remains is the trade off between shipping changes faster vs completely DRY code.

@patrickohh leaving the decision up to you. Let us know if you are done and we'll merge it as is or if you'd like to take on the challenge of implementing Jamie's clever optimization we are here for that too! 👍

Made an attempt on taking @jyaymie recommendation. But running into a format issue with the description section of the card.

Superb work on this one @patrickohh 👍

We really appreciate your willingness to take on the challenge of refactoring the html and css to be more concise than the original implementation.

Thank you for reviewing my code and offering suggestions along the way @jyaymie @MattPereira!